Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improve stability of initial state sync in Snaps UI #25375

Merged
merged 3 commits into from
Jun 18, 2024

Conversation

FrederikBolding
Copy link
Member

@FrederikBolding FrederikBolding commented Jun 18, 2024

Description

Improves stability of initial state sync when using Snaps UI. Previously, sometimes, the initialState in the SnapInterfaceContext wouldn't be updated when the content of the interface changed and thus initial values would be missing.

This PR changes the implementation to use one selector in the renderer, which only changes when the content does, but passes the initialState and context to the SnapInterfaceContext itself. This way we are guaranteed for the content and state to stay in sync.

Open in GitHub Codespaces

Can be tested with a Snap like:

export const onRpcRequest: OnRpcRequestHandler = async ({
  origin,
  request,
}) => {
  switch (request.method) {
    case 'hello': {
      const id = await snap.request({
        method: 'snap_createInterface',
        params: {
          ui: (
            <Box>
              <Text>foo</Text>
              <Button>Click me</Button>
            </Box>
          ),
          context: { foo: 'bar' },
        },
      });

      return snap.request({
        method: 'snap_dialog',
        params: {
          type: 'confirmation',
          id,
        },
      });
    }
    default:
      throw new Error('Method not found.');
  }
};

export const onUserInput: OnUserInputHandler = async ({
  id,
  event,
  context,
}) => {
  if (event.type === 'ButtonClickEvent') {
    await snap.request({
      method: 'snap_updateInterface',
      params: {
        id,
        ui: (
          <Box>
            <Heading>Settings</Heading>
            <Input name="foo" value="foo" />
            <Form name="settingsForm">
              <Field label="foo">
                <Input name="foo" value="foo" />
              </Field>
            </Form>
          </Box>
        ),
      },
    });
    return;
  }
};

@FrederikBolding FrederikBolding added the team-snaps-platform Snaps Platform team label Jun 18, 2024
@FrederikBolding FrederikBolding requested a review from a team as a code owner June 18, 2024 09:01
@FrederikBolding FrederikBolding force-pushed the fb/fix-snap-ui-initial-sync branch from bbfee15 to 7d074a0 Compare June 18, 2024 09:02
@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Jun 18, 2024
Copy link

codecov bot commented Jun 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.42%. Comparing base (3820f01) to head (f797264).
Report is 2 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #25375   +/-   ##
========================================
  Coverage    65.42%   65.42%           
========================================
  Files         1380     1380           
  Lines        54688    54688           
  Branches     14339    14339           
========================================
  Hits         35776    35776           
  Misses       18912    18912           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@metamaskbot
Copy link
Collaborator

Builds ready [f797264]
Page Load Metrics (46 ± 5 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint6010773115
domContentLoaded8251042
load398446115
domInteractive8251042
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: -22 Bytes (-0.00%)
  • common: 0 Bytes (0.00%)

@FrederikBolding FrederikBolding merged commit 45c9466 into develop Jun 18, 2024
74 checks passed
@FrederikBolding FrederikBolding deleted the fb/fix-snap-ui-initial-sync branch June 18, 2024 13:43
@github-actions github-actions bot locked and limited conversation to collaborators Jun 18, 2024
@metamaskbot metamaskbot added release-12.1.0 Issue or pull request that will be included in release 12.1.0 release-12.0.0 Issue or pull request that will be included in release 12.0.0 and removed release-12.1.0 Issue or pull request that will be included in release 12.1.0 labels Jun 18, 2024
@metamaskbot
Copy link
Collaborator

Missing release label release-12.0.0 on PR. Adding release label release-12.0.0 on PR and removing other release labels(release-12.1.0), as PR was cherry-picked in branch 12.0.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
INVALID-PR-TEMPLATE PR's body doesn't match template release-12.0.0 Issue or pull request that will be included in release 12.0.0 team-snaps-platform Snaps Platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants